Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CB-11714: (windows) added check for encoding in savePhoto() without height/width #242

Merged
merged 4 commits into from
Oct 3, 2018

Conversation

DisruptiveMind
Copy link
Contributor

Platforms affected

Windows 10

What does this PR do?

Adds extra content-type check when capturing photos without specifying targetWidth and/or targetHeight in options to preserve JPEG file extension.

What testing has been done on this change?

ditto

Checklist

  • Reported an issue in the JIRA database
  • Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
  • Added automated test coverage as appropriate for this change.

@cordova-qa
Copy link

Cordova CI Build has completed successfully.

Commit - Link
Dashboard - Link

Builder Name Console Output Test Report Device Logs
Windows 8.1 Store Link Link Link
Windows 10 Store Link Link Link
Windows 8.1 Phone Link Link Link
iOS Link Link Link
Android Link Link Link

@janpio
Copy link
Member

janpio commented Sep 16, 2018

Hey @DisruptiveMind , there seems to be a merge conflict now. Could you maybe fix this? Thanks.

@janpio janpio added the bug label Sep 17, 2018
@janpio
Copy link
Member

janpio commented Oct 1, 2018

Thanks @DisruptiveMind.

Tests are now failing because of

/Users/travis/build/apache/cordova-plugin-camera/src/windows/CameraProxy.js
  790:67  error  Strings must use singlequote  quotes
✖ 1 problem (1 error, 0 warnings)
  1 error, 0 warnings potentially fixable with the `--fix` option.

This doesn't make too much sense to me, especially as line 790 contains no quotes at all.

Can you maybe rebase on master? I hope this will go away with more recent test configuration. Thanks!

@janpio
Copy link
Member

janpio commented Oct 3, 2018

Closing and re-opening to trigger a new CI/test run with new PR merge.

@janpio janpio closed this Oct 3, 2018
@janpio janpio reopened this Oct 3, 2018
@brodycj
Copy link

brodycj commented Oct 3, 2018

Looks OK to me, merging

@brodycj brodycj merged commit dc73954 into apache:master Oct 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants